Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use skip from pytest instead of nose. #4107

Closed
wants to merge 1 commit into from

Conversation

u99127
Copy link
Contributor

@u99127 u99127 commented Oct 11, 2019

Instead of using notest from nose migrate to using skip from pytest.
Replace the check by looking for an exception where if building
with llvm older than 8.0 and skipping the test accordingly.

Ok ?

Ramana

Instead of using notest from nose migrate to using skip from pytest.
Replace the check by looking for an exception where if building
with llvm older than 8.0 and skipping the test accordingly.
@u99127
Copy link
Contributor Author

u99127 commented Oct 11, 2019

@tqchen , @mshawcroft - if you could review that would be good.

@u99127
Copy link
Contributor Author

u99127 commented Oct 11, 2019

Ok, the CI has broken because on my laptop I have llvm-4 and CI is running with llvm-8 , assuming install_cpu.sh is correct in setting the version of llvm to llvm-8.

Using floordiv instead of div for j and ak where the error appears is something that moves things forward only for me to hit an illegal instruction.

I don't understand enough of vnni to look to even fix this up and neither do I have access to the cascadelake hardware to check that this works fine. Neither do I know that CI is running on the appropriate hardware.

In general the testing infrastructure needs to now start adding run time checks for the presence of newer instructions and skip tests gracefully as we will have folks running tests on hardware that has the support for the new instructions and hardware that doesn't. I'll note this is also applicable for some of the new AArch64 dot product instructions going in.

One way I've done this in other projects is to have a dummy program containing an inline assembler instruction in the new instruction family and figuring out a way of executing that before running the test. If you get a run time exception , catch it and skip it. This routine could become common in the test framework for tvm for such hardware checks of these features and thus just be kept common.

I guess for now we either need to xfail this test or skip it entirely, unless the original contributor comes back or someone else can pick this up.

regards
Ramana

@anijain2305
Copy link
Contributor

I will try to take a stab at this next week. I am starting to look into VNNI and I have just gotten hold of a Cascade Lake machine. I will try to find out a way to skip the test by finding a suitable guard condition (LLVM intrinsic or LLVM flag etc).

@u99127
Copy link
Contributor Author

u99127 commented Oct 12, 2019

Thanks @anijain2305 , I'll let you handle it.

@tqchen
Copy link
Member

tqchen commented Oct 14, 2019

opened an issue to track the cpu feature detection problem #4120

@tqchen
Copy link
Member

tqchen commented Oct 14, 2019

#4121 provides a temporary replacement of nose skip via pytest skip. needs to followup with a feature detection code , thanks @u99127 @anijain2305 , close this thread for now

@tqchen tqchen closed this Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants